Issue/635/Modify coordinate system implementation #636
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…effects with instructions to use example data (#629) * feat: Add coordinate_system parameter to compute_tangential_and_cross_components and _compute_lensing_angles functions * Update documentation to account for new coordinate_system option * feat: Add coordinate_system conversion test to test_compute_lensing_angles_flatsky function * Add coordinate_system conversion test to test_compute_lensing_angles_astropy function and fix bug in coordinate system conversion bug in compute_lensing_angles_astropy * Set coordinate_system option during creation of GalaxyCluster object * adding a new notebook and data * test * new notebook to test coordinate systems and shear measurements * Removed debuggin print in test_dataops.py * editing README for the data availability * moved tomomi's notebook to examples * added prelude to Tomomi's notebook * Fixed typo and set default coordinate_system to "pixel" * Added option to choose coordinate system for generated mock catalog * Added ellipticity coordinate system conversion tests to test_galaxycluster.py and test_mockdata.py * Added documentation to other test cases in Tomomi's notebook. Still need to incorporate Caio's added functionality for sky vs. pixel coordinates in CLMM * Fixed formatting * starting to work through and document example of lensing signal calculation of HSCY3 cluster without CLMM, but need to change colossus dependency because it is not easy to install on NERSC * Finished baseline documentation, including changing colossus cosmology dependency in notebook to the astropy one so this is easier to run on NERSC * minor change to clarify sections of notebook * added kwargs to GalaxyCluster instance definition in preparation for re-installing latest CLMM with Caio's added coordinate_system kwargs * Updated logic for lensing angle conversion between coordinate systems in _compute_lensing_angles_flatsky and updated tests * Revert "Updated logic for lensing angle conversion between coordinate systems in _compute_lensing_angles_flatsky and updated tests" This reverts commit 4a1faa6. * Added coordinate_system validation and extra tests * Changed example notebook to document new coordinate_system option * examples/test_coordinate.ipynb has additional minor markdown detail * test_coordinate.ipynb now runs from top to bottom using Caio's coordinate system implementation! * adding additional docstrings to Tomomi's from-scratch calculation of lensing from HSC data * refactoring code into functions to make loop over cluster lenses more readable and some renaming of variables, e.g. sum to profile_sum, to avoid name space problems * made additional refactoring changes, but refactored functions to calculate angles between source and lens, etc. likely broke something in the background selection (now empty) * some of the refactoring incorporated with pre-broken code * cleaned up the loop over rbins * In line comments for portions of calculation * modified reverted commit to when the functions did not break, added in documentation and light refactoring from latest commit * renamed the notebook for consistency * docs/doc-config.ini now includes the demo_coordinate_system notebook * feat: Add coordinate_system parameter to compute_tangential_and_cross_components and _compute_lensing_angles functions (#624) * feat: Add coordinate_system parameter to compute_tangential_and_cross_components and _compute_lensing_angles functions * Update documentation to account for new coordinate_system option * feat: Add coordinate_system conversion test to test_compute_lensing_angles_flatsky function * Add coordinate_system conversion test to test_compute_lensing_angles_astropy function and fix bug in coordinate system conversion bug in compute_lensing_angles_astropy * Set coordinate_system option during creation of GalaxyCluster object * Removed debuggin print in test_dataops.py * Fixed typo and set default coordinate_system to "pixel" * Added option to choose coordinate system for generated mock catalog * Added ellipticity coordinate system conversion tests to test_galaxycluster.py and test_mockdata.py * Fixed formatting * Updated logic for lensing angle conversion between coordinate systems in _compute_lensing_angles_flatsky and updated tests * Revert "Updated logic for lensing angle conversion between coordinate systems in _compute_lensing_angles_flatsky and updated tests" This reverts commit 4a1faa6. * Added coordinate_system validation and extra tests * Changed example notebook to document new coordinate_system option * Fixed all the documentation for coordinate_system and changed names from 'pixel'/'sky' to 'euclidean'/'celestial' * Added reference for defitions of coordinate systems * Fixed bug in coordinate system conversion in mock data generation * Test ValueError raise for coordinate_system in test_mockdata.py * Added more explanations about ellipticity coordinate system * Fixed typo * uodated tag to 1.12.4 --------- Co-authored-by: Marina Ricci <[email protected]> * removed a character in a markdown cell that tests complained about in making docs * swapped pixel/euclidean and sky/celestial in notebook * reverted two files to try to match main * removed outputs in the notebook * removed OTHER notebook outputs * remove empty cells in notebooks * made minor edits in response to review, primarily with generalizing file paths and removing comments or redundant text in the markdown * incorporating original demo_mock_cluster.ipynb from main to remove changes to the file in this branch that should not have been committed * modified the Oguri example to also show incorrect coordinate assumption as noted by Caio * got rid of dollar signs inside markdown code snippet env hoping this will get the checks passed * added review contributions line :-) * added a note for the CosmoDC2 clusters so the user is aware to not necessarily expect a strong signal in center regions * removed extra cell at end * removed rogue savefig * clarified why there might be less signal in the TXPipe outputted CosmoDC2 thing * Formatted demo_coordinate_system_datasets.ipynb notebook --------- Co-authored-by: Caio Lima de Oliveira <[email protected]> Co-authored-by: Camille Avestruz <[email protected]> Co-authored-by: Marina Ricci <[email protected]>
marina-ricci
changed the title
|
@caioolivv would you like to take a look at this issue? |
|
Hi @m-aguena, yes! The last month has been a bit chaotic, but I'll start working on this this week. |
…jects, refactored tests and some bug fixes
…coordinate_system usage
|
Hi @marina-ricci and @ts485 ! In the demo notebook, there still seems to be a problem in DC2. Is that the same problem you've noticed? |
|
Hi @caioolivv , Was your problem the path to the data directory or something else? If the latter, could you tell me what the problem is? My problem is simply that Cluster-related stages in TXPipe does not run with DC2 anymore. |
The problem is in the profile: neither coordinate system creates a profile with a lensing signal. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR aims at resolving issues explained in issue #635.
Copied from the issue :
These are my suggestions :
compute_tangential_and_cross_components, from data_ops or as a GalaxyCluster method. Maybe not necessary to have it as a GalaxyCluster keyword because it may be confusing as it is not necessarily propagated.